Automate user curation #6590
Automate user curation #6590
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request automates the curation of expired users by introducing a management command that deletes users who haven't logged in for a configurable number of days, scheduling this command via cron, and updating corresponding settings.
- Implements a new management command to delete users based on their last login date.
- Schedules the command as a weekly cron job with a new decorator configuration.
- Adds new configuration settings for DMS integration and user expiration.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| kitsune/users/management/commands/cleanup_expired_users.py | Adds the new command to delete expired users based on last_login timing. |
| scripts/cron.py | Introduces a new scheduled job for running the expired user cleanup. |
| kitsune/settings.py | Updates settings with DMS_CLEANUP_EXPIRED_USERS and USER_EXPIRATION_DAYS. |
* Delete users who last logged in over X years ago, as defined in `settings.USER_EXPIRATION_DAYS` via a management command `cleanup_expired_users.py` * Run this weekly via cron * Added DMS call and DMS to settings
34da4e0 to
69fa1ca
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces an automated user curation feature that deletes users who haven't logged in for a specified period, runs the cleanup as a scheduled cron job, and adds the necessary settings and DMS integrations.
- Added a management command to delete expired users.
- Configured a new scheduled cron job to run the cleanup command weekly.
- Introduced new settings for user expiration days and a DMS call for cleanup notifications.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| kitsune/users/management/commands/cleanup_expired_users.py | Implements the user cleanup management command with deletion logging. |
| scripts/cron.py | Configures a new scheduled job to run the cleanup command via cron. |
| kitsune/settings.py | Adds new settings required for user expiration and DMS cleanup process. |
| self.stdout.write(f"Deleted user {user.username}") | ||
|
|
||
| self.stdout.write( | ||
| self.style.SUCCESS(f"Successfully processed {expired_users.count()} expired users") |
There was a problem hiding this comment.
The final success message calls expired_users.count() after users have been deleted, which may result in an incorrect count (likely 0). Consider storing the initial count before processing the deletions and using that stored value in the success message.
| self.style.SUCCESS(f"Successfully processed {expired_users.count()} expired users") | |
| self.style.SUCCESS(f"Successfully processed {expired_users_count} expired users") |
Copilot uses AI. Check for mistakes.
kitsune/settings.py
Outdated
| @@ -1331,3 +1332,5 @@ def filter_exceptions(event, hint): | |||
|
|
|||
| SUMO_BOT_USERNAME = config("SUMO_BOT_USERNAME", default="SumoBot") | |||
| SUMO_CONTENT_GROUP = config("SUMO_CONTENT_GROUP", default="Staff Content Team") | |||
|
|
|||
| USER_EXPIRATION_DAYS = config("USER_EXPIRATION_DAYS", default=1095, cast=int) | |||
There was a problem hiding this comment.
Maybe USER_INACTIVITY_DAYS?
There was a problem hiding this comment.
More descriptive - done!
|
|
||
| def handle(self, *args, **options): | ||
| User = get_user_model() | ||
| expiration_date = timezone.now() - timedelta(days=settings.USER_EXPIRATION_DAYS) |
There was a problem hiding this comment.
timezone.now() returns a datetime object. Maybe we should convert this to timezone.now().date()
There was a problem hiding this comment.
Can you explain further why this should be changed? last_login is a DateTimeField. I guess the precision of the time of day isn't important for this code, but it also doesn't cause a problem as far as I can tell?
There was a problem hiding this comment.
No it does not, it's totally valid. The reason for this nit was the naming of the variable. By converting to date() it just removes the time component which as you said it's not relevant for this kind of functionality. This is just nitpicking so treat it as such
| skip=settings.READ_ONLY, | ||
| ) | ||
| @babis.decorator(ping_after=settings.DMS_CLEANUP_EXPIRED_USERS) | ||
| def job_cleanup_expired_users(): |
There was a problem hiding this comment.
Let's add a waffle flag/switch to control when this is enabled through admin.
settings.USER_EXPIRATION_DAYSvia a management commandcleanup_expired_users.py